Skip to content

Conversation

@c21
Copy link
Contributor

@c21 c21 commented Jan 25, 2022

What changes were proposed in this pull request?

This PR is a followup of #35068 to fix the null pointer exception when calling ConstantColumnVector.close(). ConstantColumnVector.childData can be null for e.g. non-struct data type.

Why are the changes needed?

Fix the exception when cleaning up column vector.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Modified unit test in ConstantColumnVectorSuite.scala to exercise the code path of ConstantColumnVector.close() for every tested data type. Without the fix, the unit test throws NPE.

@github-actions github-actions bot added the SQL label Jan 25, 2022

@Override
public void close() {
stringData = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not related to the NPE fix, but it's good to have. We should release the memory for UTF8String as well.

@c21
Copy link
Contributor Author

c21 commented Jan 25, 2022

cc @cloud-fan and @Yaohua628

@Yaohua628
Copy link
Contributor

thanks for the fix!

Copy link
Contributor

@Yaohua628 Yaohua628 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 543e008 Jan 26, 2022
@c21
Copy link
Contributor Author

c21 commented Jan 26, 2022

Thank you @cloud-fan and @Yaohua628 for review!

@c21 c21 deleted the constant-fix branch January 26, 2022 17:24
senthh pushed a commit to senthh/spark-1 that referenced this pull request Feb 3, 2022
### What changes were proposed in this pull request?

This PR is a followup of apache#35068 to fix the null pointer exception when calling `ConstantColumnVector.close()`. `ConstantColumnVector.childData` can be null for e.g. non-struct data type.

### Why are the changes needed?

Fix the exception when cleaning up column vector.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Modified unit test in `ConstantColumnVectorSuite.scala` to exercise the code path of `ConstantColumnVector.close()` for every tested data type. Without the fix, the unit test throws NPE.

Closes apache#35324 from c21/constant-fix.

Authored-by: Cheng Su <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants